-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor codebase into installable "reprostim" Python module (+ reprostim-videocapture) package #124
Conversation
…st workflow action placeholder, #124.
"Programming Language :: Python :: Implementation :: PyPy", | ||
] | ||
dependencies = [] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define extras for psychopy and all so by default psychopy is not installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is resolved. ⬆️
…st workflow action placeholder, #124.
@candleindark please review this PR. |
pyproject.toml
Outdated
"pyaudio>=0.2.14", | ||
"reedsolo>=1.7.0", | ||
"psychopy", | ||
"psychopy-sounddevice", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: bummer can't reuse other definitions like in setup.cfg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be done differently though. See comment below.
] | ||
|
||
[project.optional-dependencies] | ||
test = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of specifying an optional dependency for testing. It would be better if a test environment is specified with the same dependency. See https://hatch.pypa.io/latest/config/environment/overview/#dependencies for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be I'll need is some details, because initially thought about this like we always have default "dependencies" and in case of optional "test" one it will add something specific to this area, like "pytest". But I can be wrong in my assumptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is nothing wrong with your current setup. However, the current setup is not taking advantage of Hatch's ability to manage environments for you.
Strictly speaking, test
is not really a feature of the app like audio
and psychopy
. It is more of an environment you that want to set up to run tests of the app. Hatch allows you to define different environments, with different dependencies, to do different things. For example, you can set up environments for static type checking, linting, and testing. All environments have the required dependencies of the project installed, and you can specified additional dependencies just for a particular environment. Run hatch env show
you will see all the available environments except the internal ones. Run hatch env show -i
you will see the internal environments use by some of the Hatch subcommands such as hatch fmt
.
My point is that instead of defining a test
feature/extra, you may want to define one or multiple test environments. With environment definitions, you can have fine control of many aspect of the environments. For examples, you can create an environment just with the required dependencies installed, an environment with a feature dependency install, and run different tests in different environments. I have created a test environment for the dandisets-linkml-status-tools
project and use it for testing. Additionally, hatch comes with a hatch-test
internal environment which you can customize for your need.
pyproject.toml
Outdated
classifiers = [ | ||
"Development Status :: 3 - Alpha", | ||
"Programming Language :: Python", | ||
"Programming Language :: Python :: 3.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we still supporting python 3.8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed older Python versions before 3.10. Actually the lowest python version "denominator" is mostly defined by psychopy. While it can run under Python 3.8, the 3.10 version is suggested - in my experiments with Linux/MacOS only Python 3.10 worked well. So let's stick to it ATM. May be with time we can allow other implementations on demand.
src/reprostim/cli/entrypoint.py
Outdated
@click.option( | ||
"-l", | ||
"--log-level", | ||
default="DEBUG", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't "INFO"
a more common default for --log-level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting default log level to "DEBUG"
seems a bit excessive to me. For example, the logs clutter simple usages as shown below.
▶ reprostim qr-parse --help
2025-01-17 13:42:23,246 [DEBUG] Logging level set to: DEBUG
2025-01-17 13:42:23,246 [DEBUG] reprostim v0.0.6
2025-01-17 13:42:23,246 [DEBUG] main(...), command=qr-parse
Usage: reprostim qr-parse [OPTIONS] PATH
Utility to parse video and locate integrated QR time codes.
Options:
--mode [PARSE|INFO] Specify execution mode. Default is "PARSE", normal
execution. Use "INFO" to dump video file info like
duration, bitrate, file size etc, (in this case "path"
argument specifies video file or directory containing
video files).
--help Show this message and exit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's good idea in terms of productization. Inspite the fact it's under beta-alpha stages, in general ideas and scope, EU should see only INFO logs. Changed/done.
I was able to build and install both the wheel and sdist targets. However, I was not able to run Running in the environment created by HatchDeveloper/Dartmouth/reprostim rf-library ✔ 13d14h
▶ hatch shell
source "/Users/isaac/Library/Application Support/hatch/env/virtual/reprostim/p_0OX6WF/reprostim/bin/activate"
Developer/Dartmouth/reprostim 13d14h
▶ source "/Users/isaac/Library/Application Support/hatch/env/virtual/reprostim/p_0OX6WF/reprostim/bin/activate"
(reprostim)
Developer/Dartmouth/reprostim rf-library ✔ 13d14h
▶ reprostim --help
Usage: reprostim [OPTIONS] COMMAND [ARGS]...
Command-line interface to run ReproStim tools and services. To see help for
the specific command, run:
reprostim COMMAND --help
e.g. reprostim timesync-stimuli --help
Options:
--version Show the version and exit.
-l, --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL]
Set the logging level.
-f, --log-format TEXT Set the logging format string. For the
pattern details see standard Python
'logging.Formatter' documentation.
--help Show this message and exit.
Commands:
detect-noscreen Utility to determine no-signal frames in...
echo Echo the provided message or the default value.
qr-parse Utility to parse video and locate integrated QR time...
timesync-stimuli PsychoPy reprostim timesync-stimuli script.
(reprostim)
Developer/Dartmouth/reprostim rf-library ✔ 13d14h
▶ which zbarimag
zbarimag not found
(reprostim)
Developer/Dartmouth/reprostim rf-library ✔ 13d14h ⍉
▶ which zbarimg
/opt/homebrew/bin/zbarimg
(reprostim)
Developer/Dartmouth/reprostim rf-library ✔ 13d14h
▶ zbarimg --version
0.23.93
(reprostim) Running in an environment created by `venv`~/Downloads
▶ python -m venv venv && source venv/bin/activate
(venv)
~/Downloads
▶ pip list
Package Version
---------- -------
pip 22.0.4
setuptools 58.1.0
WARNING: You are using pip version 22.0.4; however, version 24.3.1 is available.
You should consider upgrading via the '/Users/isaac/Downloads/venv/bin/python -m pip install --upgrade pip' command.
(venv)
~/Downloads
▶ pip install -U pip setuptools
Requirement already satisfied: pip in ./venv/lib/python3.9/site-packages (22.0.4)
Collecting pip
Using cached pip-24.3.1-py3-none-any.whl (1.8 MB)
Requirement already satisfied: setuptools in ./venv/lib/python3.9/site-packages (58.1.0)
Collecting setuptools
Using cached setuptools-75.8.0-py3-none-any.whl (1.2 MB)
Installing collected packages: setuptools, pip
Attempting uninstall: setuptools
Found existing installation: setuptools 58.1.0
Uninstalling setuptools-58.1.0:
Successfully uninstalled setuptools-58.1.0
Attempting uninstall: pip
Found existing installation: pip 22.0.4
Uninstalling pip-22.0.4:
Successfully uninstalled pip-22.0.4
Successfully installed pip-24.3.1 setuptools-75.8.0
(venv)
~/Downloads
▶ pip list
Package Version
---------- -------
pip 24.3.1
setuptools 75.8.0
(venv)
~/Downloads
▶ pip install /Users/isaac/Downloads/o/reprostim-0.0.6.tar.gz
Processing ./o/reprostim-0.0.6.tar.gz
Installing build dependencies ... done
Getting requirements to build wheel ... done
Preparing metadata (pyproject.toml) ... done
Collecting click-didyoumean>=0.3.1 (from reprostim==0.0.6)
Using cached click_didyoumean-0.3.1-py3-none-any.whl.metadata (3.9 kB)
Collecting click>=8.1.7 (from reprostim==0.0.6)
Using cached click-8.1.8-py3-none-any.whl.metadata (2.3 kB)
Collecting numpy>=1.26.4 (from reprostim==0.0.6)
Using cached numpy-2.0.2-cp39-cp39-macosx_14_0_arm64.whl.metadata (60 kB)
Collecting opencv-python>=4.9.0.80 (from reprostim==0.0.6)
Using cached opencv_python-4.11.0.86-cp37-abi3-macosx_13_0_arm64.whl.metadata (20 kB)
Collecting pydantic>=2.7.1 (from reprostim==0.0.6)
Using cached pydantic-2.10.5-py3-none-any.whl.metadata (30 kB)
Collecting pyzbar>=0.1.9 (from reprostim==0.0.6)
Using cached pyzbar-0.1.9-py2.py3-none-any.whl.metadata (10 kB)
Collecting qrcode>=8.0 (from reprostim==0.0.6)
Using cached qrcode-8.0-py3-none-any.whl.metadata (17 kB)
Collecting annotated-types>=0.6.0 (from pydantic>=2.7.1->reprostim==0.0.6)
Using cached annotated_types-0.7.0-py3-none-any.whl.metadata (15 kB)
Collecting pydantic-core==2.27.2 (from pydantic>=2.7.1->reprostim==0.0.6)
Using cached pydantic_core-2.27.2-cp39-cp39-macosx_11_0_arm64.whl.metadata (6.6 kB)
Collecting typing-extensions>=4.12.2 (from pydantic>=2.7.1->reprostim==0.0.6)
Using cached typing_extensions-4.12.2-py3-none-any.whl.metadata (3.0 kB)
Using cached click-8.1.8-py3-none-any.whl (98 kB)
Using cached click_didyoumean-0.3.1-py3-none-any.whl (3.6 kB)
Using cached numpy-2.0.2-cp39-cp39-macosx_14_0_arm64.whl (5.3 MB)
Using cached opencv_python-4.11.0.86-cp37-abi3-macosx_13_0_arm64.whl (37.3 MB)
Using cached pydantic-2.10.5-py3-none-any.whl (431 kB)
Using cached pydantic_core-2.27.2-cp39-cp39-macosx_11_0_arm64.whl (1.8 MB)
Using cached pyzbar-0.1.9-py2.py3-none-any.whl (32 kB)
Using cached qrcode-8.0-py3-none-any.whl (45 kB)
Using cached annotated_types-0.7.0-py3-none-any.whl (13 kB)
Using cached typing_extensions-4.12.2-py3-none-any.whl (37 kB)
Building wheels for collected packages: reprostim
Building wheel for reprostim (pyproject.toml) ... done
Created wheel for reprostim: filename=reprostim-0.0.6-py2.py3-none-any.whl size=67854 sha256=64fab4f488206429272ea0d76ea76051e35d975bd8c3ecb6b34a364900ab34c0
Stored in directory: /Users/isaac/Library/Caches/pip/wheels/14/62/93/52fe5e98200160b0caebe0e4c7cf9738a518ab00ddbda45bb2
Successfully built reprostim
Installing collected packages: pyzbar, typing-extensions, qrcode, numpy, click, annotated-types, pydantic-core, opencv-python, click-didyoumean, pydantic, reprostim
Successfully installed annotated-types-0.7.0 click-8.1.8 click-didyoumean-0.3.1 numpy-2.0.2 opencv-python-4.11.0.86 pydantic-2.10.5 pydantic-core-2.27.2 pyzbar-0.1.9 qrcode-8.0 reprostim-0.0.6 typing-extensions-4.12.2
(venv)
~/Downloads
▶ reprostim
Traceback (most recent call last):
File "/Users/isaac/Downloads/venv/bin/reprostim", line 5, in <module>
from reprostim.cli.entrypoint import main
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/cli/entrypoint.py", line 60, in <module>
from .cmd_qr_parse import qr_parse # noqa: E402
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/cli/cmd_qr_parse.py", line 10, in <module>
from ..qr.qr_parse import do_info, do_parse
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/qr/qr_parse.py", line 17, in <module>
from pyzbar.pyzbar import ZBarSymbol, decode
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/pyzbar.py", line 7, in <module>
from .wrapper import (
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 151, in <module>
zbar_version = zbar_function(
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 148, in zbar_function
return prototype((fname, load_libzbar()))
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 127, in load_libzbar
libzbar, dependencies = zbar_library.load()
File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/zbar_library.py", line 65, in load
raise ImportError('Unable to find zbar shared library')
ImportError: Unable to find zbar shared library
(venv)
~/Downloads ⍉
▶ which zbarimg
/opt/homebrew/bin/zbarimg
(venv)
~/Downloads
▶ zbarimg --version
0.23.93 I don't know if this is a MacOS issue. @yarikoptic If you make |
Two issues.
move import of actual functionality into the click's interface function, so we do not import any dependencies for mere
I will push some moves performed, please check the rest and adjust
edit: also pushed commit to strip double ERROR in msg |
…help and avoid crashes ATM on OSX if libzbar is not available, even running plain `reprostim` would cause a crash.
The
|
…pyproject.toml Co-authored-by: Isaac To <[email protected]>
@candleindark thanks for review. You can continue discussion or may be even create separate issues. But since @vmdocua already uploaded to pypi https://pypi.org/project/reprostim/ (0.0.7) let's merge and we will tag with 0.0.7 the 1bf01ea |
ref:
Notes.
Initial scripts to expose as entry points
Parsing/parse_wQR.py
:reprostim qr-parse
src/reprostim-capture/nosignal/reprostim/nosignal
:reprostim detect-noscreen
tools/reprostim-timesync-stimuli
:reprostim timesync-stimuli
--help
quickly etc, delay import of psychopy etc until used in the functionreprostim-capture
C/C++ CI workflow action failure caused bycatch2
libraryv2
->v3
external upgrade